Repository navigation
Ruff formatting and linting auto-fixes - #241
Merged
Merged
Conversation
Collaborator
Author
|
I'm on the test failures. |
Collaborator
|
This LGTM. I reviewed the last two steps manually. All changes seem reasonable and the fact that tests are passing is reassuring. Looking forward to reviewing the remaining issues that couldn't be auto-fixed. |
jtniehof
merged commit Oct 26, 2026
b276158
into
IMAP-Science-Operations-Center:main
16 of 17 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Performs auto-formatting and auto-linting fixes with ruff on the whole L3 codebase. Closes #240
There are
threefour phases andthreefour corresponding commits. The diffs are going to be terrible so please do verify process.Versions used:
linting
In the
imap_L3_processingcheckout, ran:I then manually checked the results with:
The results were mostly f-strings that had no substitutions (e.g.
f"foobar") and unused imports. I didn't actually verify that every removed import was unused, figuring unit tests would get that.git add -u git commit -m "Safe auto-fixes from ruff check" --no-verifyCommit e451191
formatting
The goal here was to use two characteristics of
black: 1. it is idempotent; 2. it preserves the AST of the input code. So if we runblackon the original code, and then on the code after runningruff, the result should be the same.Operating in the directory above my checkout:
405 files were changed by ruff; of them, 85 were changed differently between the two runs of black. The differences between the black-ified versions were checked by hand:
suspicious_files.txt
The post-ruff black-ified code, compared to the pre-ruff blackified code:
I identified three files to hand-fix (next section, next commit) but did not do anything in this commit--i.e. this commit is entirely the auto-fixed code. Other than those three, nothing in this check changed behavior.
There were also five files that black failed to parse before ruff ran. For these, I hand-checked the changes which ruff made:
All looked fine.
git add -u git commit -m "Auto-fixes from ruff format" --no-verifyCommit d4572b0
hand fixes
This is the commit that probably needs the closest review.
imap_l3_processing/maps/mpfit.pyhad a lot of whitespace in comments from the original Fortran. I restored these comments, changed tabs to spaces, and marked the comment blocks no-format.scripts/ultra/create_example_ultra_l2_map_with_power_law.pyandtests/codice/l3/lo/test_codice_lo_processor.pyhad lines that ended with a comma, so were turned into tuples that were never assigned to anything. I removed the tuple-wrappinggit add -u git commit -m "Hand-fix formatting" --no-verifyCommit 9ecd008
Fix mocks of unused imports
e451191 removed unused imports. However, the tests still mocked on some of the unused. There were two categories:
get_parent_file_namesfromprocessortoutils, but not all the mocks were updated. With the removal of the unused spiceypy import fromprocessor, these broke. Updated the mocks toutils.test_read_cdf_parentsintest_utils.pymockedimap_data_accessonglows.l3bc.utilsinstead ofutils. 9f5210e removed the use ofimap_data_accessfromglows.l3bc.utils, so the import was removed. Updated to useutils.Commit 3d15512 -- like the hand fixes, this is a good one to check more closely